Skip to content

Sphinx - Leidy Martinez C22#38

Open
Leidy-Martinez wants to merge 5 commits intoAda-C22:mainfrom
Leidy-Martinez:main
Open

Sphinx - Leidy Martinez C22#38
Leidy-Martinez wants to merge 5 commits intoAda-C22:mainfrom
Leidy-Martinez:main

Conversation

@Leidy-Martinez
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@mikellewade mikellewade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Your project lets me know that you have a goo understanding of React and useState!

Comment on lines +7 to +11
const calculateTotalLikeCount = (chat) => {
return chat.reduce((total, chat) => {
return total + (chat.liked ? 1 : 0);
}, 0);
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent execution!

Comment on lines +13 to +21
const localSender = messages[0].sender;
let remoteSender = 'Unknown';

for (const message of messages) {
if (message.sender !== localSender) {
remoteSender = message.sender;
break; // Stop once other sender is found
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice this handles cases of double texting!

Comment on lines +27 to +37
const handleLikeButtom = (id) => {
setChat(chat => {
return chat.map(chat => {
if (chat.id === id) {
return { ...chat, liked: !chat.liked };
} else {
return chat;
}
});
});
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No notes on this! Shows me that you have a great understanding of lifting up state!



const App =() => {
const [chat, setChat] = useState(messages);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could change this from chat to chats since it is multiple chat messages instead of one?

Comment on lines +49 to +53
<ChatLog
entries={chat}
onLiked={handleLikeButtom}
localSender={localSender}
likes={totalLikes} />}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does totalLikes actually need to be passed? We are only using it in line 44.

const ChatLog = (props) => {
const chatentrycomponents = props.entries.map((entries, index) => {
return (
<ul key={index}>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have the ids of entries we could actually use that. We want to avoid using the indexes.



const ChatLog = (props) => {
const chatentrycomponents = props.entries.map((entries, index) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could maybe name this entry instead of entries since it only a single entry we will be looking at each iteration.

Comment on lines +32 to +43
entries: PropTypes.arrayOf(
PropTypes.shape({
id: PropTypes.number.isRequired,
sender: PropTypes.string.isRequired,
body: PropTypes.string,
timeStamp: PropTypes.string.isRequired,
liked: PropTypes.bool.isRequired,
})
).isRequired,

onLiked: PropTypes.func.isRequired,
localSender: PropTypes.string.isRequired,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⭐️

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that the tests are saying that some of the props are undefined but that isn't the case. I'm not entirely sure why.

Comment on lines +7 to +9
const likeButtonCliked = () => {
props.onLiked(props.id);
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The button function could also be implemented like so:

<button className='like' onClick={() => likeButtonClicked(id)}>{liked ? '❤️': '🤍'}</button>

Comment on lines +11 to +12
const heartColor = props.liked ? '❤️' : '🤍';
const messageClass = props.sender === props.localSender ? 'local' : 'remote';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ternaries, love to see them!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants